Skip to content

Conversation

@imhayatunnabi
Copy link
Contributor

@imhayatunnabi imhayatunnabi commented Oct 13, 2025

This PR fixes a critical race condition vulnerability in the PermissionRegistrar class that affects applications running in concurrent environments (Laravel Octane, Swoole, RoadRunner, etc.).

Problem

The loadPermissions() method had a Time-of-Check to Time-of-Use (TOCTOU) race condition where multiple concurrent requests could simultaneously pass the if ($this->permissions) check before any of them loaded the permissions, causing:

  1. Cache Stampede - Multiple simultaneous expensive database queries
  2. Performance Degradation - Redundant operations under high load
  3. Memory Waste - Multiple copies of permission data in memory
  4. Database Overload - Excessive queries during cache refresh

Solution

Implemented a thread-safe double-checked locking pattern:

  • Fast path check: Quick return if permissions already loaded
  • Loading flag: $isLoadingPermissions prevents concurrent loading
  • Wait & retry: Competing threads wait 10ms and recheck
  • Double-check locking: Verify permissions not loaded by another thread
  • Exception safety: try-finally ensures flag is always released

Code Changes

File: src/PermissionRegistrar.php

  1. Added private bool $isLoadingPermissions = false; property
  2. Enhanced loadPermissions() with thread-safe implementation
  3. Updated clearPermissionsCollection() to reset the loading flag

@imhayatunnabi
Copy link
Contributor Author

@freekmurze can i get a review or merge update ?

@imhayatunnabi imhayatunnabi requested a review from drbyte October 24, 2025 04:51
@drbyte drbyte changed the title Fix critical race condition in permission loading for concurrent environments Fix TOCTOU race condition in permission loading for concurrent (Octane etc) environments Oct 27, 2025
@drbyte drbyte merged commit 7c5f4ed into spatie:main Oct 27, 2025
43 checks passed
@imhayatunnabi imhayatunnabi deleted the fix/race-condition-permission-loading branch October 29, 2025 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants